Should these two operations be in the other order?

Added a code comment.

I think I disagree with your code comment. If we're about to execute a request then we should aim to cancel the timeout first, as this will make some small number of requests succeed where the other order would fail.

Let's look at the definition of the request timeouts.

When a request timeout fires, it will tell the state machine about it having fired. The state machine will then look for the request in its own request queue. If it doesn't find the request in there, it knows that the request was already removed (probably in another thread) before. The action will for this reason be none.

For this reason the order in calling the request actions does not matter. The only place where we make decisions about failing requests is in the state machine.

From the HTTPConnectionPool:

    private func scheduleRequestTimeout(_ request: Request, on eventLoop: EventLoop) {
        let requestID = request.id
        let scheduled = eventLoop.scheduleTask(deadline: request.connectionDeadline) {
            // The timer has fired. Now we need to do a couple of things:
            //
            // 1. Remove ourselves from the timer dictionary to not leak any data. If our
            //    waiter entry still exists, we need to tell the state machine, that we want
            //    to fail the request.
            let timeoutFired = self.timerLock.withLock {
                self._requestTimer.removeValue(forKey: requestID) != nil
            }

            // 2. If the entry did not exists anymore, we can assume that the request was
            //    scheduled on another connection. The timer still fired anyhow because of a
            //    race. In such a situation we don't need to do anything.
            guard timeoutFired else { return }

            // 3. Tell the state machine about the timeout
            let action = self.stateLock.withLock {
                self._state.timeoutRequest(requestID)
            }

            self.run(action: action)
        }

        self.timerLock.withLockVoid {
            assert(self._requestTimer[requestID] == nil)
            self._requestTimer[requestID] = scheduled
        }

        request.req.requestWasQueued(self)
    }

From the StateMachine:

        mutating func timeoutRequest(_ requestID: Request.ID) -> Action {
            // 1. check requests in queue
            if let request = self.requests.remove(requestID) {
                return .init(
                    request: .failRequest(request, HTTPClientError.getConnectionFromPoolTimeout, cancelTimeout: false),
                    connection: .none
                )
            }

            // 2. This point is reached, because the request may have already been scheduled. A
            //    connection might have become available shortly before the request timeout timer
            //    fired.
            return .none
        }

Same ordering question here.

For efficiency reasons should we be able to do a "bulk" request timeout cancellation? We have to acquire locks in order to do this (both the timerLock and the event loop locks) so it may be useful to try to coalesce work where we can to try to reduce the pressure on those locks.

Can this .seconds(30) be lifted out to a static so that we can see it clearly and give it a name?

private?

Not actioned.

private?

Should we add an equivalent close API accepting an optional promise and use that instead?

Great idea. Promise is enough I guess :)

private?

Is the ordering of execute and cancelling the timeout done for any particular reason?

This API wasn't too clear to me. In my mind "makeThing" should return a "Thing" to the caller. Consider a different name / dropping a comment in here.

nit: timeoutFired / timedOut

Not actioned.

private here too.

I think I disagree with your code comment. If we're about to execute a request then we should aim to cancel the timeout first, as this will make some small number of requests succeed where the other order would fail.

Maybe just changing "make" to "create" is enough to differentiate make it clear that it's not a factory. Alternatively something like provideNewConnection(to: self, ...)?